Skip to content

Chore: Clean up extend impls for BufferMut#5055

Merged
connortsui20 merged 3 commits into
developfrom
ct/clean-extend
Oct 24, 2025
Merged

Chore: Clean up extend impls for BufferMut#5055
connortsui20 merged 3 commits into
developfrom
ct/clean-extend

Conversation

@connortsui20

@connortsui20 connortsui20 commented Oct 23, 2025

Copy link
Copy Markdown
Member

I'm moving this from #5049 since things are becoming more complicated.

The only logical change here is at the top of extend_iter:

        // Since we do not know the length of the iterator, we can only guess how much memory we
        // need to reserve. Note that these hints may be inaccurate.
        let (lower_bound, upper_bound_opt) = iter.size_hint();
        self.reserve(upper_bound_opt.unwrap_or(lower_bound));

And the other is now using offset_from_unsigned (used to be called ptr_sub on nightly)

@connortsui20 connortsui20 requested review from a10y and gatesn October 23, 2025 19:21
@connortsui20 connortsui20 added the changelog/chore A trivial change label Oct 23, 2025
@codspeed-hq

codspeed-hq Bot commented Oct 23, 2025 •
edited
Loading

Copy link
Copy Markdown

CodSpeed Performance Report

Merging #5055 will degrade performances by 20.37%

Comparing ct/clean-extend (587a2b5) with develop (f5c1c70)

Summary

❌ 8 regressions
✅ 1306 untouched

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark BASE HEAD Change
❌ patched_take_200k_dispersed 4.7 ms 5.7 ms -16.48%
❌ patched_take_200k_first_chunk_only 4.8 ms 5.4 ms -10.59%
❌ take_200k_dispersed 3.7 ms 4.6 ms -19.14%
❌ take_200k_first_chunk_only 3.4 ms 4.3 ms -20.37%
❌ filter_runend[(1000, 16, 0.01)] 22.3 µs 24.8 µs -10.1%
❌ filter_runend[(1000, 256, 0.005)] 19.4 µs 21.9 µs -11.47%
❌ filter_runend[(1000, 256, 0.01)] 19.3 µs 21.8 µs -11.5%
❌ filter_runend[(1000, 256, 0.03)] 19.3 µs 21.8 µs -11.5%

@codecov

codecov Bot commented Oct 23, 2025 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 66.66667% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.45%. Comparing base (f5c1c70) to head (587a2b5).
⚠️ Report is 1 commits behind head on develop.

Files with missing lines Patch % Lines
vortex-buffer/src/buffer_mut.rs 66.66% 10 Missing ⚠️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

gatesn
gatesn reviewed Oct 23, 2025
View reviewed changes
Comment thread vortex-buffer/src/buffer_mut.rs Outdated
@connortsui20 connortsui20 mentioned this pull request Oct 23, 2025
Closed
@connortsui20 connortsui20 mentioned this pull request Oct 23, 2025
Closed
gatesn
gatesn reviewed Oct 23, 2025
View reviewed changes
Comment thread vortex-buffer/src/buffer_mut.rs Outdated
I: TrustedLen<Item = T>,
{
// We allow the `extend_trusted` method to correctly allocate the required memory.
let mut buffer = Self::with_capacity(0);

@gatesn gatesn Oct 23, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's actually more efficient to pass the correct capacity here from the trusted iter, otherwise we do two allocations instead of one.

@connortsui20 connortsui20 Oct 24, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is related: #5044, putting this here for the link

@connortsui20
clean up extend impls for BufferMut
8e8fa8b 
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
@connortsui20
enforce hard limit on size_hint upper bound
acaaa2f 
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
@connortsui20
remove weird alignment check
587a2b5 
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
@connortsui20 connortsui20 enabled auto-merge (squash) October 24, 2025 14:45
@connortsui20 connortsui20 merged commit bfdc6d3 into develop Oct 24, 2025
63 of 66 checks passed
@connortsui20 connortsui20 deleted the ct/clean-extend branch October 24, 2025 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@robert3005 robert3005 robert3005 approved these changes

@gatesn gatesn gatesn approved these changes

@a10y a10y Awaiting requested review from a10y

Assignees

No one assigned

Labels

changelog/chore A trivial change

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@connortsui20 @robert3005 @gatesn